Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

#

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Jump to bottom

fix: Use SPA Dataset link in Chart and Dashboard #20941

Merged
merged 4 commits into from
Aug 3, 2022
Merged

fix: Use SPA Dataset link in Chart and Dashboard #20941

merged 4 commits into from
Aug 3, 2022

Conversation

EugeneTorap
Copy link
Contributor

Improve #20890 PR logic and use dataset links in SPA in Chart and Dashboard

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

Screenshot 2022-08-02 at 09 30 41

Screenshot 2022-08-02 at 09 53 47

TESTING INSTRUCTIONS

ADDITIONAL INFORMATION

  • Has associated issue:
  • Required feature flags:
  • Changes UI
  • Includes DB Migration (follow approval process in SIP-59)
    • Migration is atomic, supports rollback & is backwards-compatible
    • Confirm DB migration upgrade and downgrade tested
    • Runtime estimates and downtime expectations provided
  • Introduces new feature or API
  • Removes existing feature or API
    @michael-s-molina @kgabryje

@pull-request-size pull-request-size bot added size/S and removed size/XS labels Aug 2, 2022
@codecov
Copy link

codecov bot commented Aug 2, 2022 •
edited
Loading

Codecov Report

Merging #20941 (f38a740) into master (bce32af) will increase coverage by 0.00%.
The diff coverage is 50.00%.

@@           Coverage Diff           @@
##           master   #20941   +/-   ##
=======================================
  Coverage   66.38%   66.38%           
=======================================
  Files        1766     1766           
  Lines       67224    67225    +1     
  Branches     7135     7136    +1     
=======================================
+ Hits        44625    44626    +1     
  Misses      20774    20774           
  Partials     1825     1825
Flag Coverage Δ
javascript 52.11% <50.00%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
...dashboard/components/AddSliceCard/AddSliceCard.tsx 65.90% <0.00%> (-1.54%) ⬇️
...perset-frontend/src/views/CRUD/chart/ChartList.tsx 59.13% <100.00%> (ø)
superset-frontend/src/featureFlags.ts 100.00% <0.00%> (+33.33%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us.

kgabryje
kgabryje approved these changes Aug 2, 2022
View reviewed changes
Copy link
Member

@kgabryje kgabryje left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lgtm, well spotted!

michael-s-molina
michael-s-molina approved these changes Aug 2, 2022
View reviewed changes
Copy link
Member

@michael-s-molina michael-s-molina left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thank you for the improvement @EugeneTorap!

@kgabryje kgabryje changed the title Use SPA Dataset link in Chart and Dashboard fix: Use SPA Dataset link in Chart and Dashboard Aug 2, 2022
@lyndsiWilliams
Copy link
Member

I've just merged a fix for this failing test on CI. If you pull from master your CI should pass 😁

@EugeneTorap
Copy link
Contributor Author

I've just merged a fix for this failing test on CI. If you pull from master your CI should pass 😁

Done ☺️

@michael-s-molina Can we merge the PR?

@kgabryje kgabryje merged commit 96a63bc into apache:master Aug 3, 2022
@EugeneTorap EugeneTorap deleted the fix/dataset-link-in-spa-for-chart-and-dashboard branch August 3, 2022 07:28
@mistercrunch mistercrunch added 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 2.1.0 and removed 🚢 2.1.3 labels Mar 13, 2024
# for free to join this conversation on GitHub. Already have an account? # to comment
Reviewers

@kgabryje kgabryje kgabryje approved these changes

@michael-s-molina michael-s-molina michael-s-molina approved these changes

Assignees
No one assigned
Labels
🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels size/S 🚢 2.1.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@EugeneTorap @lyndsiWilliams @kgabryje @michael-s-molina @mistercrunch